perf: swap order of escape regexes to avoid lookahead #546
Conversation
|
Nice. prom-client still support Node.js 10 though, and Did you benchmark just reversing the two regexps so that a look |
|
Totally forgot that we used to live in a world without From the microbenchmark, it seems like the swapped regex is as fast, if not faster (!!!). Somewhat surprising given that https://jsbench.me/qklez5ombn/1 I'm happy to just swap the regex order and call it a day given that it seems to be the winner anyway. The thing that really matters is that these operations (original regex vs. swapped w/o lookahead) are equivalent; am I missing anything? FWIW I did make sure that on my test string ( |
ngavalas
changed the title
|
I'll update the changelog after all the checks come back green to avoid an extra roundtrip if something's still wrong. My bad. :) |
|
Awesome, easy win. Yes, the behavior from just swapping the regexps looks correct. |
* swap regex order to avoid lookahead * changelog * changelog formatting apparently
* swap regex order to avoid lookahead * changelog * changelog formatting apparently
* swap regex order to avoid lookahead * changelog * changelog formatting apparently
* swap regex order to avoid lookahead * changelog * changelog formatting apparently
* swap regex order to avoid lookahead * changelog * changelog formatting apparently
* swap regex order to avoid lookahead * changelog * changelog formatting apparently
When looking at some CPU profiles of metric -> string serialization, I noticed that escaping all of the strings and labels was showing up in the flamegraph more than I would expect. This isn't a massive win by any means, but it's pretty consistently ~20% faster on a microbenchmark: https://jsbench.me/qklez5ombn/1. I'll take 20%.
Some notes:
\before replacing the\nfixes the case that made you need the.replace(/\\(?!n)/g, '\\\\')regex in the first place, so now we can just do\\->\\\\and\n->\\nand"->\\"in that order without lookaheads.Relevant to #543.